load da_committee from genesis file #3634

pls148 · 2025-10-01T22:47:56Z

This PR:

Adds support for genesis -> da_committees
Parsing of da_committees
EpochCommittees now holds da_committees as data instead of known_da_nodes

This PR does not:

Actually do anything with the new data

Key places to review:

Example da_committees in data/genesis/X.toml:

[da_committees]
0.3 = {
    0 = [
        { stake_table_key = "BLS_VER_KEY~bQszS-QKYvUij2g20VqS8asttGSb95NrTu2PUj0uMh1CBUxNy1FqyPDjZqB29M7ZbjWqj79QkEOWkpga84AmDYUeTuWmy-0P1AdKHD3ehc-dKvei78BDj5USwXPJiDUlCxvYs_9rWYhagaq-5_LXENr78xel17spftNd5MA1Mw5U", state_ver_key = "SCHNORR_VER_KEY~lJqDaVZyM0hWP2Br52IX5FeE-dCAIC-dPX7bL5-qUx-vjbunwe-ENOeZxj6FuOyvDCFzoGeP7yZ0fM995qF-CRE", stake = 1 }
    ]
}

ss-es · 2025-10-02T03:18:39Z

crates/hotshot/types/src/lib.rs

+            [(TYPES::Epoch::new(0), self.known_da_nodes.clone())].into()
+        } else {
+            if !self.known_da_nodes.is_empty() {
+                tracing::warn!(


I think this could be an error

or arguably even a panic -- I think it might be preferable to call this a misconfiguration and immediately panic vs. try to resolve the ambiguity

ss-es · 2025-10-02T03:21:02Z

types/src/v0/impls/stake_table.rs

+        let da_members = da_committees
+            .get(&Epoch::new(0))
+            .cloned()
+            .unwrap_or_default(); // For testing reasons, we want to support being given an empty map


or_default means return an empty vec here right? I think it would be better to panic, since an empty DA committee is always a (fatal) error

ss-es · 2025-10-02T03:23:37Z

sequencer/src/genesis.rs

    #[serde(default)]
    pub upgrades: BTreeMap<Version, Upgrade>,
+    #[serde(default)]
+    pub da_committees: Option<BTreeMap<TomlKeyU64, Vec<PeerConfigData>>>,


A BTreeMap can be null right? do we need to make this an Option?

also is there a reason it needs to be in both HotShotConfig and Genesis?

do we use it out of HotShotConfig anywhere? (I think it's fine to leave it in both even if we only use it out of genesis, just wondering)

ss-es · 2025-10-06T05:20:44Z

types/src/v0/config.rs

    known_nodes_with_stake: Vec<PeerConfig<SeqTypes>>,
    known_da_nodes: Vec<PeerConfig<SeqTypes>>,
+    #[serde(default)]
+    da_committees: BTreeMap<Version, BTreeMap<u64, Vec<PeerConfig<SeqTypes>>>>,


I think the inner BTreeMap is unnecessary -- this should just be a BTreeMap<Version, (u64, PeerConfig<SeqTypes>)>

maybe @bfish713 can comment, but I think it's unlikely that we'd schedule an upgrade with multiple DA committees that take effect at different points in the future? and it definitely complicates the logic

ss-es · 2025-10-07T04:23:47Z

sequencer/src/genesis.rs

+#[derive(Clone, Debug, Deserialize, Serialize)]
+pub struct VersionedDaCommittee {
+    #[serde(with = "version_ser")]
+    pub start_version: Version,
+    pub start_epoch: u64,
+    pub committee: Vec<PeerConfigData>,
+}
+


I feel like this should be imported from hotshot-types instead of re-defined

ss-es · 2025-10-07T04:26:00Z

sequencer/src/lib.rs

+        tracing::warn!("setting da_committees from genesis");
+        network_config.config.da_committees = da_committees
+            .iter()
+            .map(|src| VersionedDaCommittee::<SeqTypes> {
+                start_version: src.start_version,
+                start_epoch: src.start_epoch,
+                committee: src
+                    .committee
+                    .iter()
+                    .map(|pcd| hotshot_types::PeerConfig {
+                        stake_table_entry: StakeTableEntry {
+                            stake_key: pcd.stake_table_key,
+                            stake_amount: U256::from(pcd.stake),
+                        },
+                        state_ver_key: pcd.state_ver_key.clone(),
+                    })
+                    .collect(),
+            })
+            .collect();
+    }


I'm not sure I see the reason for any of this -- can't we just use the same type? can't this entire block just be network_config.config.da_committees = da_committees

ss-es · 2025-10-07T04:32:08Z

crates/hotshot/types/src/lib.rs

+
+    pub fn build_da_committees(&self) -> Vec<PeerConfig<TYPES>> {
+        // TODO: THIS IS A TEMPORARY FIX WITH THE WRONG RETURN TYPE.
+        // It's done so that we can start using this function and have the existing behavior
+        // (use known_da_nodes) while we transition to using da_committees.
+        self.known_da_nodes.clone()
+    }


I don't think this function is necessary. We can keep the base da_stake_table (or whatever Membership currently stores) separate from the epoch_da_committees that apply at specific epochs, as we do for the full quorum stake table anyway

ss-es · 2025-10-07T04:34:00Z

sequencer/src/lib.rs

    let mut membership = EpochCommittees::new_stake(
        network_config.config.known_nodes_with_stake.clone(),
-        network_config.config.known_da_nodes.clone(),
+        network_config.config.build_da_committees(),


I think this can (should) remain unchanged

ss-es

I think there's an unused struct that should be deleted, but other than that looks good to me

ss-es · 2025-10-10T00:21:15Z

sequencer/src/genesis.rs

+#[derive(Clone, Debug, Deserialize, Serialize)]
+pub struct PeerConfigData {
+    pub stake_table_key: <SeqTypes as NodeType>::SignatureKey,
+    pub state_ver_key: <SeqTypes as NodeType>::StateSignatureKey,
+    pub stake: u64,
+}
+


is this struct now unused? if so we should delete it

pls148 requested review from sveitser, jbearer, imabdulbasit, ss-es, bfish713, rob-maron and lukaszrzasik as code owners October 1, 2025 22:47

pls148 force-pushed the ps/load-da-committee branch 9 times, most recently from 9c100c2 to 99589b4 Compare October 2, 2025 01:17

ss-es reviewed Oct 2, 2025

View reviewed changes

pls148 force-pushed the ps/load-da-committee branch 3 times, most recently from 10bf649 to ca9dcbd Compare October 4, 2025 00:33

ss-es reviewed Oct 6, 2025

View reviewed changes

pls148 added 5 commits October 6, 2025 13:52

load da_committee from genesis file

7ab3920

fixes

0f33ed3

fix test failures

2ba49dd

change da_committees to a map of version to map of epoch to committee

ebd970c

switch to vec of versioneddacommittee

a09bb19

pls148 force-pushed the ps/load-da-committee branch from ca9dcbd to a09bb19 Compare October 6, 2025 22:02

pls148 mentioned this pull request Oct 7, 2025

Call add_da_committee on upgrade decide #3638

Open

ss-es reviewed Oct 7, 2025

View reviewed changes

remove build_da_committees

d9c4e11

fixes

1bd7291

ss-es approved these changes Oct 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

load da_committee from genesis file #3634

load da_committee from genesis file #3634

Uh oh!

pls148 commented Oct 1, 2025 •

edited

Loading

Uh oh!

ss-es Oct 2, 2025

Uh oh!

ss-es Oct 2, 2025

Uh oh!

ss-es Oct 2, 2025

Uh oh!

ss-es Oct 2, 2025

Uh oh!

ss-es Oct 6, 2025 •

edited

Loading

Uh oh!

ss-es Oct 7, 2025

Uh oh!

ss-es Oct 7, 2025

Uh oh!

ss-es Oct 7, 2025

Uh oh!

ss-es Oct 7, 2025

Uh oh!

ss-es left a comment

Uh oh!

ss-es Oct 10, 2025

Uh oh!

Uh oh!

load da_committee from genesis file #3634

Are you sure you want to change the base?

load da_committee from genesis file #3634

Uh oh!

Conversation

pls148 commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR:

This PR does not:

Key places to review:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ss-es Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ss-es left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pls148 commented Oct 1, 2025 •

edited

Loading

ss-es Oct 6, 2025 •

edited

Loading